This is failing tests because it doesn't look like the setlayout pass is running at all to reset the layout to none after trivial is found to not be perfect. In the test that's failing csplayout doesn't converge so layout is never getting reset and dense layout is skipped because we still have the trivial layout, so the trivial layout passes through to the end (which breaks the expected result).

I'm not sure how to get layout set to None in the property set based on the conditional output of a previous analysis pass. I tried to do it in the condition logic, but the fencing got it the way of that approach.

This is failing tests because it doesn't look like the setlayout pass is running at all to reset the layout to none after trivial is found to not be perfect. In the test that's failing csplayout doesn't converge so layout is never getting reset and dense layout is skipped because we still have the trivial layout, so the trivial layout passes through to the end (which breaks the expected result).

I'm not sure how to get layout set to None in the property set based on the conditional output of a previous analysis pass. I tried to do it in the condition logic, but the fencing got it the way of that approach.

Reading through the conditions here has become pretty tricky, so I'm not 100% sure I'm parsing the situation correctly. If we want to extend the _not_perfect_yet behavior from level1, it seems the best the way to do it is to add another Layout2qDistance with something like property_name='csp_layout_score' following CSPLayout, and gate _choose_layout_2 on the result of that.

Otherwise, we have a mix of cases where sometimes property_set['layout'] being non-None means "skip all future layout passes" and sometimes it doesn't. Better to avoid the confusion that's likely to arise here and re-run Layout2qDistance (and we can re-evaluate if there are better ways for the layout passes to signal to one another if they've found a perfect layout.)

This is failing tests because it doesn't look like the setlayout pass is running at all to reset the layout to none after trivial is found to not be perfect. In the test that's failing csplayout doesn't converge so layout is never getting reset and dense layout is skipped because we still have the trivial layout, so the trivial layout passes through to the end (which breaks the expected result).
I'm not sure how to get layout set to None in the property set based on the conditional output of a previous analysis pass. I tried to do it in the condition logic, but the fencing got it the way of that approach.

Reading through the conditions here has become pretty tricky, so I'm not 100% sure I'm parsing the situation correctly. If we want to extend the _not_perfect_yet behavior from level1, it seems the best the way to do it is to add another Layout2qDistance with something like property_name='csp_layout_score' following CSPLayout, and gate _choose_layout_2 on the result of that.

Otherwise, we have a mix of cases where sometimes property_set['layout'] being non-None means "skip all future layout passes" and sometimes it doesn't. Better to avoid the confusion that's likely to arise here and re-run Layout2qDistance (and we can re-evaluate if there are better ways for the layout passes to signal to one another if they've found a perfect layout.)

It's a little more involved than that because we can't rely on Layout2qDistance not being perfect after csp layout because we want to use csp's output if the pass converged on a solution and trivial wasn't perfect. If trivial isn't perfect and csp didn't converge then we should use dense layout. But this put me on the right track, instead of using layout2qdistance I looked for the csp stop reason and used that as the condition for running dense. See: fb10b69

"best of" passmanager flow controller (see PR #3018) is a similar solution. Do you mind if I tag on hold this one and try to put that PR on its feet soonish?

"best of" passmanager flow controller (see PR #3018) is a similar solution. Do you mind if I tag on hold this one and try to put that PR on its feet soonish?

I'd rather we leave it as is, if the best of PR merges before this I can refactor this PR on top of it to switch the logic to use that instead.

I'm removing the stable backport tag on this, mostly because I think we should let it sit on master a bit so we can get some benchmark runs to see the impact of this since we're plannign a release shortly. If it all looks good in a couple weeks we can add the tag back and include it for a 0.17.2 release.